Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Frontend] Add OpenAI Vision API Support #5237

Merged
merged 54 commits into from
Jun 7, 2024
Merged

Conversation

ywang96
Copy link
Member

@ywang96 ywang96 commented Jun 4, 2024

@ywang96
Copy link
Member Author

ywang96 commented Jun 4, 2024

@DarkLight1337 This PR is probably good for first pass - I will mark it as "Ready for review" after I address your first round of review, along with adding more test and documentation.

Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hard work! I have left a few comments.

tests/entrypoints/test_openai_vision.py Outdated Show resolved Hide resolved
vllm/entrypoints/openai/serving_chat.py Outdated Show resolved Hide resolved
vllm/entrypoints/openai/serving_chat.py Outdated Show resolved Hide resolved
vllm/entrypoints/openai/serving_chat.py Outdated Show resolved Hide resolved
vllm/envs.py Outdated Show resolved Hide resolved
vllm/multimodal/utils.py Outdated Show resolved Hide resolved
vllm/multimodal/utils.py Outdated Show resolved Hide resolved
vllm/entrypoints/openai/serving_chat.py Show resolved Hide resolved
@DarkLight1337 DarkLight1337 mentioned this pull request Jun 5, 2024
2 tasks
@ywang96
Copy link
Member Author

ywang96 commented Jun 5, 2024

With docs and tests modified - going to make this PR ready for review cc @DarkLight1337.

@ywang96 ywang96 marked this pull request as ready for review June 5, 2024 02:52
@ywang96 ywang96 assigned ywang96 and DarkLight1337 and unassigned ywang96 Jun 5, 2024
@DarkLight1337
Copy link
Member

DarkLight1337 commented Jun 5, 2024

Assuming that the tests pass, LGTM. I will double-check the docs after they have been built.

@ywang96
Copy link
Member Author

ywang96 commented Jun 6, 2024

Thanks for the review @simon-mo! I have addressed the comments and all related tests have passed. Could you take a look at the aiohttp implementation and let me know if there's any issue? Thanks!

@simon-mo
Copy link
Collaborator

simon-mo commented Jun 7, 2024

  • can you add the package to requirements
  • can you confirm the test is ran in ci (the entrypoints tests config in pipeline is complex)

@DarkLight1337
Copy link
Member

DarkLight1337 commented Jun 7, 2024

  • can you add the package to requirements

aiohttp was already added to common requirements in #4794.

  • can you confirm the test is ran in ci (the entrypoints tests config in pipeline is complex)

I have downloaded the CI log and confirm that this is the case (with the help of Ctrl+F).

@ywang96 ywang96 merged commit 7a9cb29 into vllm-project:main Jun 7, 2024
103 checks passed
dtrifiro pushed a commit to opendatahub-io/vllm that referenced this pull request Jun 10, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
robertgshaw2-neuralmagic pushed a commit to neuralmagic/nm-vllm that referenced this pull request Jun 11, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
joerunde pushed a commit to joerunde/vllm that referenced this pull request Jun 17, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jun 27, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 8, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 24, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
Temirulan pushed a commit to Temirulan/vllm-whisper that referenced this pull request Sep 6, 2024
Co-authored-by: DarkLight1337 <tlleungac@connect.ust.hk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants